[libcxxabi][cmake] Account for LIBCXXABI_TARGET_SUBDIR in test config #138527
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes the logic and code structure match that of libc++, which handles this case.
The `%{target}` substitution from libc++ is removed as libc++abi's config seems to be the only place it's used.
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxxabi Author: Raul Tambre (tambry) ChangesThis makes the logic and code structure match that of libc++, which handles this case (i.e. the target subdirectory being changed). Full diff: https://github.com/llvm/llvm-project/pull/138527.diff 3 Files Affected:
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index fc34009d0a551..adfb2a9f69508 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -170,7 +170,6 @@ def getSuitableClangTidy(cfg):
[
AddFeature("target={}".format(triple)),
AddFlagIfSupported("--target={}".format(triple)),
- AddSubstitution("%{triple}", triple),
],
),
),
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 6dcfc51e55321..3057d7ccbf412 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -191,6 +191,8 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LIBCXXABI_TARGET_SUBDIR})
set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LIBCXXABI_TARGET_SUBDIR} CACHE STRING
"Path where built libc++abi libraries should be installed.")
+ set(LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR "${CMAKE_INSTALL_INCLUDEDIR}/${LIBCXXABI_TARGET_SUBDIR}/c++/v1" CACHE STRING
+ "Path where target-specific libc++abi headers should be installed.")
unset(LIBCXXABI_TARGET_SUBDIR)
else()
if(LLVM_LIBRARY_OUTPUT_INTDIR)
@@ -202,6 +204,8 @@ else()
endif()
set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX} CACHE STRING
"Path where built libc++abi libraries should be installed.")
+ set(LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR "${LIBCXXABI_INSTALL_INCLUDE_DIR}" CACHE STRING
+ "Path where target-specific libc++abi headers should be installed.")
endif()
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in
index edf80c8f2a732..6b38517f052c7 100644
--- a/libcxxabi/test/configs/cmake-bridge.cfg.in
+++ b/libcxxabi/test/configs/cmake-bridge.cfg.in
@@ -30,7 +30,7 @@ config.substitutions.append(('%{libcxx}', '@LIBCXXABI_LIBCXX_PATH@'))
config.substitutions.append(('%{install-prefix}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@'))
config.substitutions.append(('%{include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/include'))
config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_DIR@'))
-config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/include/%{triple}/c++/v1'))
+config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR@'))
config.substitutions.append(('%{lib}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_LIBRARY_DIR@'))
config.substitutions.append(('%{benchmark_flags}', ''))
|
|
@llvm/pr-subscribers-libcxx Author: Raul Tambre (tambry) ChangesThis makes the logic and code structure match that of libc++, which handles this case (i.e. the target subdirectory being changed). Full diff: https://github.com/llvm/llvm-project/pull/138527.diff 3 Files Affected:
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index fc34009d0a551..adfb2a9f69508 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -170,7 +170,6 @@ def getSuitableClangTidy(cfg):
[
AddFeature("target={}".format(triple)),
AddFlagIfSupported("--target={}".format(triple)),
- AddSubstitution("%{triple}", triple),
],
),
),
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 6dcfc51e55321..3057d7ccbf412 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -191,6 +191,8 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LIBCXXABI_TARGET_SUBDIR})
set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LIBCXXABI_TARGET_SUBDIR} CACHE STRING
"Path where built libc++abi libraries should be installed.")
+ set(LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR "${CMAKE_INSTALL_INCLUDEDIR}/${LIBCXXABI_TARGET_SUBDIR}/c++/v1" CACHE STRING
+ "Path where target-specific libc++abi headers should be installed.")
unset(LIBCXXABI_TARGET_SUBDIR)
else()
if(LLVM_LIBRARY_OUTPUT_INTDIR)
@@ -202,6 +204,8 @@ else()
endif()
set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX} CACHE STRING
"Path where built libc++abi libraries should be installed.")
+ set(LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR "${LIBCXXABI_INSTALL_INCLUDE_DIR}" CACHE STRING
+ "Path where target-specific libc++abi headers should be installed.")
endif()
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
diff --git a/libcxxabi/test/configs/cmake-bridge.cfg.in b/libcxxabi/test/configs/cmake-bridge.cfg.in
index edf80c8f2a732..6b38517f052c7 100644
--- a/libcxxabi/test/configs/cmake-bridge.cfg.in
+++ b/libcxxabi/test/configs/cmake-bridge.cfg.in
@@ -30,7 +30,7 @@ config.substitutions.append(('%{libcxx}', '@LIBCXXABI_LIBCXX_PATH@'))
config.substitutions.append(('%{install-prefix}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@'))
config.substitutions.append(('%{include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/include'))
config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_DIR@'))
-config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/include/%{triple}/c++/v1'))
+config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_INCLUDE_TARGET_DIR@'))
config.substitutions.append(('%{lib}', '@LIBCXXABI_TESTING_INSTALL_PREFIX@/@LIBCXXABI_INSTALL_LIBRARY_DIR@'))
config.substitutions.append(('%{benchmark_flags}', ''))
|
tambry
commented
May 5, 2025
|
Ping |
|
Ping. I'll go ahead and merge this sometime next week. |
|
LGTM post-commit, sorry for missing this review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes the logic and code structure match that of libc++, which handles this case (i.e. the target subdirectory being changed).
The
%{target}substitution from libc++ is removed as libc++abi's config seems to be the only place it's used.